Add row tracking preservation tests for MERGE, UPDATE and DELETE on OSS Delta [databricks] by jtwynne · Pull Request #15907 · NVIDIA/cudf-spark

jtwynne · 2026-09-06T19:28:27Z

Description

The only tests that check the GPU DML commands preserve row tracking ran on Databricks 17.3. The OSS UPDATE and DELETE commands preserve it through UpdateCommand.preserveRowTrackingColumns, and the OSS MERGE command inherits ClassicMergeExecutor.writeAllChanges, which does the same, but nothing exercised them (#15906 first claimed a defect there; it was wrong, the coverage gap is what remains).

supports_delta_lake_row_tracking() in spark_session.py: Databricks 17.3 and later, or OSS Delta Lake 3.3, which the plugin pairs with Spark 3.5.x and later.
test_delta_update_preserves_row_tracking and test_delta_delete_preserves_row_tracking (renamed from _db173) are gated on it instead of Databricks 17.3.
test_delta_merge_preserves_row_tracking: a matched update and an insert into a row-tracked target, run on both engines through assert_gpu_and_cpu_writes_are_equal_collect. For a Delta test the helper runs the GPU side under assert_rapids_delta_write, which asserts the GPU Delta write in the plans it captures, and only the GPU merge command writes that way, so a fallback to the CPU merge command fails the test. The helper then reads both tables back on the CPU with _metadata.row_id and _metadata.row_commit_version and compares them engine to engine (both tables start from the same single file, so the rows that existed before carry the same ids and versions on both engines; the inserted row's id is masked because the file layout decides it and the join-based GPU merge lays out files differently from the CPU). Per engine, the rows that existed before keep their row id, copied rows keep their commit version, the updated row gets the new one, and the inserted row gets a fresh id and a commit version past the pre-merge ones. The merge's result row is compared between the engines; the commit logs are not, for the same layout reason. The NOT MATCHED BY SOURCE clause is left out because it runs on the GPU only with Delta 4.1 and Databricks 17.3; Support NOT MATCHED BY SOURCE in the GPU MERGE command on Databricks 17.3 [databricks] #15884 has the 17.3 test with that clause.

The capability predicate includes Databricks 17.3, where UPDATE and DELETE preserve row tracking on the GPU and the two tests pass. The 17.3 GPU MERGE on main regenerates row ids (#15884 fixes that and carries the 17.3 merge test), so test_delta_merge_preserves_row_tracking is skipped on Databricks with that reason until #15884 lands.

Tests

On an Azure Standard_NC16as_T4_v3 (one T4, driver 610.57.04), plugin built from this branch, run_pyspark_from_build.sh --delta_lake -m delta_lake -k preserves_row_tracking with the Delta packages, extension and catalog confs from jenkins/spark-tests.sh:

Apache Spark 4.0.0, Scala 2.13, Delta Lake 4.0.1 (build version 400): 3 passed (test_delta_merge_preserves_row_tracking, test_delta_update_preserves_row_tracking, test_delta_delete_preserves_row_tracking), 0 failed, no WAS LEAKED lines.
Apache Spark 3.5.6, Scala 2.12, Delta Lake 3.3.0 (build version 356): the same 3 passed, 0 failed, no WAS LEAKED lines.
Apache Spark 4.1.1, Scala 2.13, Delta Lake 4.1.0 (build version 411): the same 3 passed, 0 failed, no WAS LEAKED lines.
Re-run after the merge test moved to the standard helper: 3 passed, 0 failed, no WAS LEAKED lines on each of Spark 4.0.0 / Delta 4.0.1, 3.5.6 / 3.3.0 and 4.1.1 / 4.1.0 (same T4 box).
Databricks 17.3 LTS ML (Standard_NC16as_T4_v3, run_pyspark_from_build.sh --runtime_env=databricks): test_delta_update_preserves_row_tracking and test_delta_delete_preserves_row_tracking passed, test_delta_merge_preserves_row_tracking skipped with the Support NOT MATCHED BY SOURCE in the GPU MERGE command on Databricks 17.3 [databricks] #15884 reason, no WAS LEAKED lines.

Checklists

Documentation

Updated for new or modified user-facing features or behaviors
No user-facing change

Testing

Added or modified tests to cover new code paths
Covered by existing tests
(Please provide the names of the existing tests in the PR description.)
Not required

Performance

Tests ran and results are added in the PR description
Issue filed with a link in the PR description
Not required

…SS Delta The only tests that check the GPU DML commands preserve row tracking ran on Databricks 17.3. The OSS UPDATE and DELETE commands preserve it through UpdateCommand.preserveRowTrackingColumns and the OSS MERGE command inherits ClassicMergeExecutor.writeAllChanges, which does the same, but nothing exercised them. Add supports_delta_lake_row_tracking: Databricks 17.3 and later, or OSS Delta Lake 3.3, which the plugin pairs with Spark 3.5.x and later. Gate the UPDATE and DELETE row tracking tests on it instead of Databricks 17.3 and drop the _db173 suffix from their names. Add test_delta_merge_preserves_row_tracking: a matched update and an insert into a row-tracked target on the CPU and on the GPU; the rows that existed before keep their row id on both engines, copied rows keep their commit version, the updated row gets the new one, and the inserted row gets a fresh id. The ids are checked per row because the join-based merge lays out files differently from the CPU, which the commit-log comparison of the UPDATE and DELETE tests does not tolerate. The NOT MATCHED BY SOURCE clause is left out because it runs on the GPU only with Delta 4.1 and Databricks 17.3. Fixes NVIDIA#15906 Signed-off-by: Thomas Wynne <jtwynne3@gmail.com>

greptile-apps · 2026-09-06T19:30:22Z

Confidence Score: 5/5

The PR appears safe to merge, with no outstanding correctness or repository-rule issues identified.

Summary

Adds a shared capability predicate for OSS Delta 3.3+/Spark 3.5+ and Databricks 17.3+.
Broadens UPDATE and DELETE row-tracking tests to supported OSS runtimes.
Adds a MERGE test covering preserved IDs, commit-version changes, and fresh metadata for inserted rows.
Uses the standard CPU/GPU write-comparison helper with GPU Delta execution verification.
Skips MERGE on Databricks until the separately tracked row-ID regeneration fix lands.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Create identical row-tracked CPU and GPU tables] --> B[Capture metadata before MERGE]
    B --> C[Run CPU MERGE]
    B --> D[Run GPU MERGE with Delta execution verification]
    C --> E[Read tracked rows on CPU]
    D --> F[Read tracked rows on CPU]
    E --> G[Compare CPU and GPU results]
    F --> G
    G --> H[Verify existing row IDs are preserved]
    H --> I[Verify commit versions change only where expected]
    I --> J[Verify inserted row receives fresh tracking metadata]

The merge row tracking test ran the GPU merge through the plan-capture helper and compared the business columns between the engines by hand. Use assert_gpu_and_cpu_writes_are_equal_collect instead: it runs the merge on both engines and compares the rows that existed before, including their row id and commit version, which are identical on both engines because both tables start from the same single file. The per-engine checks stay for what cannot be compared across engines: the ids and versions against the before state, and the freshness of the inserted row's id, which the file layout decides. Signed-off-by: Thomas Wynne <jtwynne3@gmail.com>

Run the merge on both engines through assert_cpu_and_gpu_are_equal_collect_with_capture with GpuExecutedCommandExec as a required class, so a fallback to the CPU merge command fails the test instead of passing on identical results, and compare the merge's own result row between the engines. The rows that existed before are still compared engine to engine with their row id and commit version, and the per-engine checks against the before state and for the inserted row's fresh id are unchanged. Signed-off-by: Thomas Wynne <jtwynne3@gmail.com>

The previous commit asserted GpuExecutedCommandExec on the merge's own DataFrame plan. Since Spark 3.5 that plan is a CommandResult wrapper around the executed command, so the assertion could not see the GPU command and failed on Spark 3.5.6 although the GPU merge had run. Capture the plans of the GPU run with ExecutionPlanCaptureCallback, as the REORG tests do, and require GpuExecutedCommandExec and the GPU Delta write in them. The merge result row and the rows that existed before, with their row id and commit version, are still compared between the engines. Signed-off-by: Thomas Wynne <jtwynne3@gmail.com>

… skip it on Databricks test_delta_merge_preserves_row_tracking drove the CPU and GPU sessions by hand with its own plan capture. It now runs through assert_gpu_and_cpu_writes_are_equal_collect, the path the UPDATE and DELETE tests already take: for a Delta test the helper runs the GPU side under assert_rapids_delta_write, which asserts the GPU Delta write that only the GPU merge command produces, then reads both tables back on the CPU with the row tracking columns and compares them engine to engine (the inserted row's id masked, since the file layout decides it). The merge's result row and the per-engine row id and commit version checks stay. The capability predicate includes Databricks 17.3, but the 17.3 GPU merge on main regenerates row ids (NVIDIA#15884 fixes that and carries the 17.3 test), so the MERGE test is skipped on Databricks with that reason. The UPDATE and DELETE tests keep running on 17.3. Signed-off-by: Thomas Wynne <jtwynne3@gmail.com>

greptile-apps · 2026-09-07T17:05:22Z

Want your agent to iterate on Greptile's feedback? Try greploops.

firestarman

LGTM, two nits.

firestarman · 2026-09-08T05:46:23Z

build

firestarman

Thanks a lot for submitting this PR!

test_delta_merge_preserves_row_tracking checked that the inserted row's id is past every id the table held before the merge; it now checks its commit version the same way, next to that check, so the row tracking semantics of an insert are asserted on each engine and not only through the CPU/GPU comparison. The test's new string formatting (the tracked-rows query, the masked read and the assertion messages) uses f-strings. Ran on the T4 box: 3 passed, 0 leaks on each of Spark 4.0.0 / Delta 4.0.1, 3.5.6 / Delta 3.3.0 and 4.1.1 / Delta 4.1.0. Signed-off-by: Thomas Wynne <jtwynne3@gmail.com>

…nne/cudf-spark into oss-merge-row-tracking-test

firestarman · 2026-09-09T01:45:39Z

build

firestarman

LGTM

firestarman · 2026-09-09T03:39:42Z

build

gerashegalov · 2026-09-09T03:57:16Z

+                    reason="The Databricks 17.3 GPU merge regenerates row ids until #15884 lands; that PR carries the 17.3 test")
+def test_delta_merge_preserves_row_tracking(spark_tmp_path):
+    # A matched update and an insert touch a row-tracked target. (A NOT MATCHED BY SOURCE clause
+    # runs on the GPU only with Delta 4.1 and Databricks 17.3, so it is left out here.) The row


Add a Delta 4.1-specific variant containing WHEN NOT MATCHED BY SOURCE ... UPDATE, then verify that the target-only row retains its row ID and receives an advanced commit version.

This was referenced Sep 6, 2026

Support NOT MATCHED BY SOURCE in the GPU MERGE command on Databricks 17.3 [databricks] #15884

Open

[TASK] Add row tracking preservation tests for the GPU MERGE on OSS Delta 3.3 and 4.x #15906

Open

jtwynne added 4 commits September 6, 2026 14:43

Merge branch 'main' into oss-merge-row-tracking-test

4317644

greptile-apps Bot reviewed Sep 7, 2026

View reviewed changes

Comment thread integration_tests/src/main/python/delta_lake_merge_test.py Outdated

Comment thread integration_tests/src/main/python/delta_lake_merge_test.py

jtwynne changed the title ~~Add row tracking preservation tests for MERGE, UPDATE and DELETE on OSS Delta~~ Add row tracking preservation tests for MERGE, UPDATE and DELETE on OSS Delta [databricks] Sep 7, 2026

jtwynne added 4 commits September 6, 2026 22:09

Merge branch 'main' into oss-merge-row-tracking-test

234afd5

Merge branch 'main' into oss-merge-row-tracking-test

2efbde5

Merge branch 'main' into oss-merge-row-tracking-test

a8e4549

sameerz added the test Only impacts tests label Sep 8, 2026

sameerz requested a review from a team September 8, 2026 01:39

firestarman reviewed Sep 8, 2026

View reviewed changes

Comment thread integration_tests/src/main/python/delta_lake_merge_test.py Outdated

Comment thread integration_tests/src/main/python/delta_lake_merge_test.py

firestarman previously approved these changes Sep 8, 2026

View reviewed changes

jtwynne added 2 commits September 8, 2026 13:18

Merge branch 'oss-merge-row-tracking-test' of https://github.com/jtwy…

2a77e59

…nne/cudf-spark into oss-merge-row-tracking-test

jtwynne dismissed firestarman’s stale review via 2a77e59 September 8, 2026 18:18

jtwynne requested a review from firestarman September 8, 2026 19:17

Merge branch 'main' into oss-merge-row-tracking-test

24341d7

firestarman approved these changes Sep 9, 2026

View reviewed changes

Merge branch 'main' into oss-merge-row-tracking-test

97daf5a

gerashegalov requested changes Sep 9, 2026

View reviewed changes

Navigation Menu

Uh oh!

Add row tracking preservation tests for MERGE, UPDATE and DELETE on OSS Delta [databricks] - #15907

Add row tracking preservation tests for MERGE, UPDATE and DELETE on OSS Delta [databricks]#15907
jtwynne wants to merge 13 commits into
NVIDIA:mainfrom
jtwynne:oss-merge-row-tracking-test

jtwynne commented Sep 6, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Sep 6, 2026 •

edited

Loading

Summary

Diagram

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot commented Sep 7, 2026

Uh oh!

firestarman left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

firestarman commented Sep 8, 2026

Uh oh!

firestarman left a comment

Uh oh!

firestarman commented Sep 9, 2026

Uh oh!

firestarman left a comment

Uh oh!

firestarman commented Sep 9, 2026

Uh oh!

gerashegalov Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

jtwynne commented Sep 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Tests

Checklists

Uh oh!

greptile-apps Bot commented Sep 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 5/5

Summary

Diagram

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot commented Sep 7, 2026

Uh oh!

firestarman left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

firestarman commented Sep 8, 2026

Uh oh!

firestarman left a comment

Choose a reason for hiding this comment

Uh oh!

firestarman commented Sep 9, 2026

Uh oh!

firestarman left a comment

Choose a reason for hiding this comment

Uh oh!

firestarman commented Sep 9, 2026

Uh oh!

gerashegalov Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jtwynne commented Sep 6, 2026 •

edited

Loading

greptile-apps Bot commented Sep 6, 2026 •

edited

Loading

firestarman left a comment •

edited

Loading